-
Notifications
You must be signed in to change notification settings - Fork 1k
Unify jdbc tests between javaagent and library instrumentation #15137
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
f58e9d4 to
a253c52
Compare
9436596 to
fefd71f
Compare
10efdf0 to
c0f03e4
Compare
c0f03e4 to
9d2feb2
Compare
2d63bcd to
6e4b8c0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only change compared to the old javaagent PreparedStatementParametersTest is calling wrap()
| if (init != null) { | ||
| init.accept(ds); | ||
| } | ||
| Class<?> originalDatasourceClass = ds.getClass(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
capturing the original class here before to use in verification later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only changes compared to the old javaagent JdbcInstrumentationTest are calling wrap() and a couple of other minor changes noted by review comments
| return Stream.of( | ||
| Arguments.of( | ||
| new JdbcDataSource(), | ||
| (Consumer<DataSource>) ds -> ((JdbcDataSource) ds).setURL(jdbcUrls.get("h2")), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this changed from setUrl in the javaagent test to setURL in this module because in the javaagent module, h2 version is overridden for slick test
| try { | ||
| ds.setDriverClass(jdbcDriverClassNames.get(dbType)); | ||
| } catch (PropertyVetoException e) { | ||
| throw new IllegalStateException(e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this changed from RuntimeException to IllegalStateException b/c it's now in src/main and so errorprone rules apply
|
|
||
| @SuppressWarnings("deprecation") // using deprecated semconv | ||
| class DruicDataSourceTest { | ||
| class DruidDataSourceTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(unrelated fix)
| private String url; | ||
| Consumer<String> sqlConsumer = unused -> {}; | ||
|
|
||
| public TestConnection() {} | ||
| private final String url; | ||
| private final Consumer<String> sqlConsumer; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unrelated simplification
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed tests from here that are now covered by the shared tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only change compared to the old javaagent PreparedStatementParametersTest is calling wrap()
No description provided.